bug(fix): PhoneNumberField does not honor readonly prop #1878
Conversation
🦋 Changeset detectedLatest commit: bd13a85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const dialList = dialCodeList ?? countryDialCodes; | ||
| const countryCodeOptions = React.useMemo( | ||
| () => | ||
| dialList.map((dialCode) => ( | ||
| <option key={dialCode} value={dialCode}> | ||
| <option key={dialCode} value={dialCode} disabled={isReadOnly}> |
There was a problem hiding this comment.
Should we add a comment of why we are mapping disabled to isReadOnly? We will wonder later on.
| ); | ||
|
|
||
| return ( | ||
| <SelectField | ||
| aria-disabled={isReadOnly} |
There was a problem hiding this comment.
Same here, I think there should be a comment on why this is more accessible.
| @@ -29,6 +31,34 @@ describe('PhoneNumberField primitive', () => { | |||
| }; | |||
| }; | |||
|
|
|||
| const originalLog = console.log; | |||
There was a problem hiding this comment.
nit: put this above the describe, and reassign originalLog in an afterEach.
| @@ -135,4 +165,24 @@ describe('PhoneNumberField primitive', () => { | |||
|
|
|||
| expect(onCountryCodeChange).toHaveBeenCalled(); | |||
| }); | |||
|
|
|||
| it('should set aria-disabled="true" when the isReadOnly prop is passed, and disable all the select options', async () => { | |||
There was a problem hiding this comment.
Add comment on why this behavior exists
I think it's because select elements don't support the https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly |
Description of changes
This PR address this issue: "PhoneNumberField dial code dropdown doesn't honor readonly prop"
As a result of these changes, if the
isReadOnlyprop is passed to thePhoneNumberField:CountryCodeSelectprimitive is marked with thearia-disabled=“true”attribute. The reason we’re not marking it witharia-readonly=“true”is because that does not cause screen readers to announce anything to customers. @hbuchelCountryCodeSelectare marked with thedisabledattribute because the select field should still be readable and focusable, but disallow changing the readonly selection.isReadOnlyprop is passed.Issue #, if available
Fixes #1795
Description of how you validated changes
Ran the docs locally and wrote some unit tests.
Checklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.